Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add backup url parts to list #2675

Merged
merged 8 commits into from
Jun 13, 2024
Merged

Conversation

vasco-santos
Copy link
Contributor

@vasco-santos vasco-santos commented Jun 11, 2024

Uses same approach and naming (parts) as web3.storage changes: web3-storage/web3.storage#2347

Closes #2674

Copy link

cloudflare-workers-and-pages bot commented Jun 11, 2024

Deploying nft-storage with  Cloudflare Pages  Cloudflare Pages

Latest commit: d90eccb
Status: ✅  Deploy successful!
Preview URL: https://8ef24951.nft-storage-1at.pages.dev
Branch Preview URL: https://fix-add-backup-url-parts-to.nft-storage-1at.pages.dev

View logs

@vasco-santos vasco-santos force-pushed the fix/add-backup-url-parts-to-list branch from 198e6df to 62c5328 Compare June 11, 2024 12:50
* @param {unknown[]} backupUrls
* @returns {Iterable<string>}
*/
function carCidV1Base32sFromBackupUrls(backupUrls) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*
* @param {string} key
*/
const bucketKeyToPartCID = (key) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think should be fine looking at the code but worth a double check - it needs to now also account for backup URL in this format:

backupUrls.push(new URL(`https://w3s.link/ipfs/${stat.cid}`))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to add a test

Comment on lines +25 to +31
// get hash links to CARs that contain parts of this upload
/** @type {string[]} */
const parts = [
// from upload table 'backup_urls' column
...carCidV1Base32sFromBackupUrls(upload.backup_urls ?? []),
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adapted from https://github.com/web3-storage/web3.storage/blob/main/packages/db/utils.js#L19

in nft.storage we ran the migration to get backUrls from backup table into upload table: #1664

Migration: https://gist.github.com/yocontra/4ae719b42a54c4582ca28fcac74199d0

I double checked a few rows of backup table and their content is now also in upload table

@@ -58,7 +58,6 @@ export class LinkdexApi {
if (!res || !res.body) throw new Error(`failed to get CAR: ${cid}`)
const carBlocks = await CarBlockIterator.fromIterable(res.body)
for await (const block of carBlocks) {
// @ts-expect-error block types not match up
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all the mess of versioning of CID in a project stalled 😭

@@ -174,6 +176,7 @@ async function unixFsEncodeDir(files, bs) {
async function unixFsEncodeString(str, bs) {
const content = new TextEncoder().encode(str)
const ic = { path: '', content }
// @ts-expect-error different CID versions
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all the mess of versioning of CID in a project stalled 😭

@@ -160,6 +161,7 @@ async function unixFsEncodeDir(files, bs) {
const content = new Uint8Array(await f.arrayBuffer())
input.push({ path: f.name, content })
}
// @ts-expect-error different CID versions
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all the mess of versioning of CID in a project stalled 😭

@@ -111,6 +111,7 @@ async function cborEncode(value, bs) {
const bytes = CBOR.encode(value)
const digest = await sha256.digest(bytes)
const cid = CID.createV1(CBOR.code, digest)
// @ts-expect-error different CID versions
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all the mess of versioning of CID in a project stalled 😭

@@ -41,7 +41,7 @@
"it-last": "^2.0.0",
"linkdex": "^3.0.0",
"merge-options": "^3.0.4",
"multiformats": "^9.6.4",
"multiformats": "^13.1.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needed to upgrade to use Link, which made needed to upgrade IPLD deps to decrease TS issues

@vasco-santos vasco-santos marked this pull request as ready for review June 11, 2024 13:06
Copy link
Contributor

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one minor point to check...

*
* @param {string} key
*/
const bucketKeyToPartCID = (key) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think should be fine looking at the code but worth a double check - it needs to now also account for backup URL in this format:

backupUrls.push(new URL(`https://w3s.link/ipfs/${stat.cid}`))

@vasco-santos vasco-santos merged commit c1cd467 into main Jun 13, 2024
22 checks passed
@vasco-santos vasco-santos deleted the fix/add-backup-url-parts-to-list branch June 13, 2024 15:57
vasco-santos pushed a commit that referenced this pull request Jun 13, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.8.1](api-v4.8.0...api-v4.8.1)
(2024-06-13)


### Bug Fixes

* add backup url parts to list
([#2675](#2675))
([c1cd467](c1cd467))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add backup URLs to upload list
2 participants